Repository navigation
Fix crashes, wrong outputs and lowering failures in several backends and in exir - #24222
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24222
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ❌ 1 New Failure, 70 PendingAs of commit 204bbc8 with merge base 254635c ( NEW FAILURE - The following job has failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@claude /pr-review |
|
Claude finished @shoumikhin's task in 22m 42s —— View job Approve: nothing blocks. The main open point is that on OpenVINO before 2026.4, the delegate blob now stays in memory next to OpenVINO's own copy for the whole life of the method. Three smaller robustness and test gaps are noted below. PR Review: #24222SummaryThe PR does two things:
Nothing blocks merging. The OpenVINO memory cost on versions before 2026.4 is worth settling, and three smaller points follow. The PyTorch Inductor and OpenVINO sources are not in this checkout, so I mark statements about their internals as low confidence. I ran no tests. Code Quality
Testing
Performance
RecommendationApprove Nothing here blocks the merge. The one point to settle is the OpenVINO memory cost: either free the blob on versions before 2026.4, or document the cost. That can happen here or in a follow-up.
|
35f7db1 to
55b4840
Compare
|
Thanks for the review. Points 1, 2 and 4 are fixed in the commits that own the code. Point 3 has no code change, see below.
|
55b4840 to
52e8dcf
Compare
52e8dcf to
c39d57f
Compare
c39d57f to
d11bd4b
Compare
9136f25 to
204bbc8
Compare
|
@pytorchbot cherry-pick --onto release/1.6 -c critical |
Cherry picking #24222The cherry pick PR is at #24298 and it is recommended to link a critical cherry pick PR with an issue. Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot cherry-pick --onto release/1.6 -c critical |
Cherry picking #24222Command Details for Dev Infra teamRaised by workflow job |
This branch was picked before #24263 and #24295 reached release/1.6, so three files carried their content only in part. Take release/1.6 and use the versions a clean pick of 054de1f gives for backends/qualcomm/partition/qnn_partitioner.py, backends/qualcomm/tests/test_passes.py and exir/tests/test_pass_infra.py.
…fer check (#24299) ## What is wrong today Robot policies and follow-up checks on #24222 found these bugs in the CUDA backend and the runtime: 1. **CUDA mask indexing:** `x[mask]` and `x[mask] = values` have no C shims, so lowering fails with "missing fallback kernels". This blocks SmolVLA. 2. **CUDA graphs:** a channels-last output turns CUDA graphs off, although a raw copy is correct. 3. **CUDA graph shapes:** when the captured call has a new batch size, it and every replay return the previous output shape. 4. **Output buffers:** `Method::set_output_data_ptr` checks a buffer against the output's current size. After a dynamic output shrinks, a too-small buffer is accepted and a later, larger call writes past it. ## What this change does One commit per fix: 1. New shims for index and index_put follow ATen's indexing rules. The Windows import library gains the new names. Inside a CUDA graph capture, a mask index fails with a clear error. 2. The check compares output strides with the strides the ETensor's dim order gives. 3. The capture step resizes each output to the captured shape. 4. The buffer is checked against the output's upper bound. Code that binds a smaller buffer after a call now gets `InvalidArgument`. ## Tests Each new test fails without its fix. - **CUDA (A100):** mask reads and writes, also inside a capture; the raw-copy check; a capture call with a new batch size. - **Runtime:** a too-small output buffer is refused after a dynamic output shrinks; the runtime tests pass. On LeRobot policies, SmolVLA now lowers to CUDA and runs in 43 ms per call on an A100, against 504 ms in eager PyTorch (cosine similarity 0.99996).
…fer check (#24299) ## What is wrong today Robot policies and follow-up checks on #24222 found these bugs in the CUDA backend and the runtime: 1. **CUDA mask indexing:** `x[mask]` and `x[mask] = values` have no C shims, so lowering fails with "missing fallback kernels". This blocks SmolVLA. 2. **CUDA graphs:** a channels-last output turns CUDA graphs off, although a raw copy is correct. 3. **CUDA graph shapes:** when the captured call has a new batch size, it and every replay return the previous output shape. 4. **Output buffers:** `Method::set_output_data_ptr` checks a buffer against the output's current size. After a dynamic output shrinks, a too-small buffer is accepted and a later, larger call writes past it. ## What this change does One commit per fix: 1. New shims for index and index_put follow ATen's indexing rules. The Windows import library gains the new names. Inside a CUDA graph capture, a mask index fails with a clear error. 2. The check compares output strides with the strides the ETensor's dim order gives. 3. The capture step resizes each output to the captured shape. 4. The buffer is checked against the output's upper bound. Code that binds a smaller buffer after a call now gets `InvalidArgument`. ## Tests Each new test fails without its fix. - **CUDA (A100):** mask reads and writes, also inside a capture; the raw-copy check; a capture call with a new batch size. - **Runtime:** a too-small output buffer is refused after a dynamic output shrinks; the runtime tests pass. On LeRobot policies, SmolVLA now lowers to CUDA and runs in 43 ms per call on an A100, against 504 ms in eager PyTorch (cosine similarity 0.99996).
…ork suite pytorch#24222 fixed I64toI32 casting a lifted int64 constant before its definition. Add the matching subtest to test_i64_to_i32.
…ion stays on CPU pytorch#24222 drops HTP partitions without runtime inputs. Cover the case of an int64 buffer whose I64toI32 cast is left without a delegated consumer.
release/1.6 regenerated aoti_cuda_shims.lib to add aoti_torch_dtype_element_size, and this branch regenerated it to add aoti_torch_as_strided. Regenerate it per aoti_cuda_shims.lib.md with both names, so it lists exactly the shims this tree exports.
Regenerate aoti_cuda_shims.lib per aoti_cuda_shims.lib.md so it lists exactly the shims this tree exports: release/1.6's set, including aoti_torch_dtype_element_size, plus aoti_torch_as_strided, aoti_torch_cuda_index_Tensor, aoti_torch_get_size and aoti_torch_index_put_out.
The pick was made on release/1.6, which does not have #24298 (the #24222 pick) yet. #24294 builds on #24222, so the pick depends on code that release/1.6 lacks, such as _nodes_written_in_place in exir's constant_prop_pass, and its conflict resolution brought in some of #24222's OpenVINO changes but not others. This merges a clean pick of 7b14148 onto #24298's branch and takes the clean pick's side in the OpenVINO files, so that, on top of #24298, the change is exactly #24294 (7b14148). A plain merge of the two would keep two copies of _PRECISIONS in preprocess.py.
This pick is stacked on #24298 (the #24222 pick), which has since gained follow-up commits for the Qualcomm partitioner and the TensorRT guide. Taking them keeps this branch equal to #24298 plus #24294, so that refreshing it from release/1.6 after #24298 merges cannot undo them. The merge is clean, and the follow-ups touch no OpenVINO files.
Fixes found while exporting models to ExecuTorch backends.
Fixes
constant_prop_passkeeps tensors that in-place ops write, also through views or inno_grad/autocastblocks.to_backendkeeps the output node last.to_edgelowerstorch._check_tensor_allandtorch.sym_ite.GroupBasedPartitionerno longer makes partitions that depend on each other, unless caller node groups already do. Tensor constants get static shapes; deduplicated constants keep their data.detachlowers, also insidetorch.cond. Strided outputs are copied. Transposed convolutions (1-D, 2-D, no dilation) andtorch._check_tensor_alllower.catrespects scoped configs. ELU is delegated when XNNPACK can run it.MatmulToBmmPasstakes a constant operand; Vulkan constants above 4 dims; non-strict programs in the NXP quantizer;cummin.out.Behavior changes
.pteloads only the weights it was exported with. A.ptdwith other weights, for example after fine-tuning, needs a newly exported.pte.GroupBasedPartitioneris about 2x slower on large graphs, and some graphs get a different number of partitions.to_backendkeeps the original order of top-level nodes.to_backend, fail at export instead of giving wrong results. Delegated ELU flushes subnormal values to zero. Cats outside a scoped quantizer config are no longer quantized.We would like to cherry-pick this PR to release/1.6 after it merges. It needs #23915 there first.
Testing
Each fix has a test that fails without it. Run on an A100 (CUDA, Vulkan), an x86 CPU (OpenVINO, QNN SDK), macOS (Core ML, MLX), the TOSA reference model and VGF emulation (Arm), and the eIQ Neutron SDK (NXP). Not run: Metal, ROCm, Windows CUDA.